Skip to content

new chapter with examples of diagnostic translation PRs #1621

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
wants to merge 2,025 commits into
base: master
Choose a base branch
from

Conversation

tshepang
Copy link
Member

I found other examples overwhelming, as they tend to handle large parts of rustc crates, so decided to point to these more simple ones I created.

tshepang and others added 30 commits July 19, 2022 09:56
Co-authored-by: Yuki Okushi <jtitor@2k36.org>
rust-lang#1406)

Co-authored-by: Yuki Okushi <jtitor@2k36.org>
Co-authored-by: Ridwan Abdilahi <riabdila@microsoft.com>
Signed-off-by: Yuki Okushi <jtitor@2k36.org>
Signed-off-by: Yuki Okushi <jtitor@2k36.org>
Signed-off-by: Yuki Okushi <jtitor@2k36.org>
Signed-off-by: Yuki Okushi <jtitor@2k36.org>
Signed-off-by: Yuki Okushi <jtitor@2k36.org>
Co-authored-by: Yuki Okushi <jtitor@2k36.org>
Co-authored-by: Joshua Nelson <github@jyn.dev>
* make date-check lightweight

This avoids having to write the date twice when updating date-check.

Before "As of <-- 2022-07 --> July 2022"
After "As of July 2022"

* please clippy

* update date-check docs

* accept review suggestion

Co-authored-by: Noah Lev <camelidcamel@gmail.com>

* address review comment

rust-lang#1394 (review)

* accept review suggestion

Co-authored-by: Noah Lev <camelidcamel@gmail.com>

* address review comment

rust-lang#1394 (review)

* address review comment

rust-lang#1394 (comment)

* this breaks markdown

* address review comment

rust-lang#1394 (comment)

This led to a more robust regex, though making the tool more picky.
It also found a wrong date format that was missed.

* address review comment

rust-lang#1394 (comment)

* address review comment

rust-lang#1394 (comment)

* accept review suggestion

This was reverted by mistake

Co-authored-by: Noah Lev <camelidcamel@gmail.com>

* address review comment

rust-lang#1394 (comment)

* use a more simple fn

* address review comment

rust-lang#1394 (comment)

Much more clean

* nit

* accept review suggestion

Co-authored-by: Noah Lev <camelidcamel@gmail.com>

* avoid a failed regex

Also, test new shape

* adjust to new regex (which uses named groups)

New regex was introduced by 456008c

Co-authored-by: Noah Lev <camelidcamel@gmail.com>
This diagram is based on the diagram in Joshua Nelson's talk on
bootstrapping at RustConf 2022 [1]. I converted it to Mermaid and made
some tweaks to simplify it and bring it closer to bootstrap's
terminology, and then Ralf Jung added nodes for copying artifacts.

[1]: https://rustconf.com/schedule#bootstrapping-the-once-and-future-compiler

Co-authored-by: Joshua Nelson <github@jyn.dev>
Co-authored-by: Ralf Jung <post@ralfj.de>
Co-authored-by: Noah Lev <camelidcamel@gmail.com>
Also, make link to upstream llvm repo clickable
@JohnTitor JohnTitor added the S-waiting-on-review Status: this PR is waiting for a reviewer to verify its content label Mar 4, 2023
Copy link
Member

@compiler-errors compiler-errors left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this. I think we could use this as an opportunity to provide a bit more "motivation" for the various parts of migrating these diagnostic structs if we add more information. Left a bunch of nits..

ecx.struct_span_err(span, "proc-macro derive produced unparseable tokens").emit();
```

> Note that `ecx.struct_span_err` is the [Session::struct_span_err].
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
> Note that `ecx.struct_span_err` is the [Session::struct_span_err].
> Note that `ecx.struct_span_err` calls [Session::struct_span_err].

This should also probably link to ecx.struct_span_err (https://doc.rust-lang.org/nightly/nightly-rustc/rustc_expand/base/struct.ExtCtxt.html#method.struct_span_err) so that users can attest this relationship themselves.

Copy link
Member Author

@tshepang tshepang Mar 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, that should have been ultimately calls...

- Replace the code above with this:

```rust
ecx.sess.emit_err(errors::ProcMacroDeriveTokens { span });
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should point to emit_err in rustdocs. Also, useful to point out that if the user wants to make a diagnostic, but not emit it, they can use create_err.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tough balance between adding useful info and avoiding the overwhelm... I should find a way to fit create_err in the reference

ecx.sess.emit_err(errors::ProcMacroDeriveTokens { span });
```

- Create the above type, `errors::ProcMacroDeriveTokens`,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- Create the above type, `errors::ProcMacroDeriveTokens`,
- Add a diagnostic struct, `errors::ProcMacroDeriveTokens`,

Referring to the [general comments](#general-comments) section above,
follow these changes:

- Replace the code above with this:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- Replace the code above with this:
- Replace the calls to `struct_span_err` and `emit` with:

Comment on lines +57 to +60
#[derive(Diagnostic)]
#[diag(expand_proc_macro_derive_tokens)]
pub struct ProcMacroDeriveTokens {
#[primary_span]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it may be useful to motivate the three important parts of this:

  • derive(Diagnostic)
  • diag(FLUENT_SLUG)
  • and primary_span

Maybe why we need these three components, what other components are common, etc?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I more wanted to have people see the pattern, than do wall-of-text which can overwhelm. The more complete details are in the reference chapters (earlier in the guide).

@tshepang tshepang added S-waiting-on-author Status: this PR is waiting for additional action by the OP and removed S-waiting-on-review Status: this PR is waiting for a reviewer to verify its content labels Jul 7, 2023
@spastorino
Copy link
Member

@compiler-errors do you think this is fine to merge now?.

@jieyouxu
Copy link
Member

jieyouxu commented Nov 2, 2024

Update: cf rust-lang/rust#132181 (diag infra currently in limbo, no longer mandatory usage, but examples/clarifications will still be very helpful)

@jieyouxu jieyouxu added A-diagnostics Area: diagnostics A-translation Area: diagnostics translation and other translations T-compiler Relevant to compiler team labels Nov 4, 2024
@Kobzol
Copy link
Contributor

Kobzol commented Jan 5, 2025

Sorry, due to me messing up a git operation, we sadly had to force-push the whole commit history of rustc-dev-guide :( If you'd like to update this pull request, you will have to rebase it in a special way onto the new commit history (the new master):

git fetch origin --all
git checkout <pr-branch>
git rebase --onto origin/master origin/master-old
git push --force-with-lease

More context can be found here.

@jieyouxu jieyouxu added S-stale Status: this PR is stale, maybe it can be salvaged/revived? and removed S-waiting-on-author Status: this PR is waiting for additional action by the OP labels May 29, 2025
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-diagnostics Area: diagnostics A-translation Area: diagnostics translation and other translations S-stale Status: this PR is stale, maybe it can be salvaged/revived? T-compiler Relevant to compiler team
Projects
None yet
Development

Successfully merging this pull request may close these issues.